bin/cli/src/commands/start-command.ts (2)

53-53: Use join() for path construction.

For consistency with the rest of the file and best practices, use join() from node:path instead of string interpolation.

♻️ Suggested fix

-  const logsPath = `${process.cwd()}/logs`;
+  const logsPath = join(process.cwd(), "logs");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` at line 53, The code constructs
logsPath using string interpolation ("const logsPath =
`${process.cwd()}/logs`;") which is brittle; change it to use path.join
(imported from 'node:path' or 'path') and set logsPath = join(process.cwd(),
'logs') and add the join import if missing so path separators are handled
correctly across platforms and consistent with other usages in this file.

---

17-30: Add JSDoc documentation for the public function.

As per coding guidelines, public APIs should have JSDoc comments. Consider adding documentation similar to the other exported functions.

📝 Suggested documentation

+/**
+ * Creates a filtered copy of the genesis file with validator keys removed.
+ * `@param` genesisPath - Path to the source genesis.json file
+ * `@returns` Path to the temporary filtered genesis file
+ */
 export async function createFilteredGenesis(genesisPath: string): Promise<string> {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 17 - 30, The exported
function createFilteredGenesis lacks JSDoc; add a JSDoc block above the
createFilteredGenesis declaration that documents the function purpose,
parameters, return value and behavior (mention it reads a genesis JSON, removes
keys listed in GENESIS_STATE_DROP_KEYS from genesis.genesis_state if present,
writes a temp file with formatted JSON, and returns the temp path), and include
`@param` {string} genesisPath and `@returns` {Promise<string}> annotations and any
thrown errors or side effects for consistency with other exported functions.

bin/cli/index.ts (1)

29-38: Consider adding start to the examples list.

The help text examples include new, build, test, and deploy, but not the new start command.

📝 Suggested update

 Examples:
   $ jammin new [options]
   $ jammin build [options]
   $ jammin test [options]
   $ jammin deploy [options]
+  $ jammin start

 For more information, visit: https://fluffylabs.dev/jammin/`,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/index.ts` around lines 29 - 38, The help text added via
program.addHelpText currently lists example commands (new, build, test, deploy)
but omits the new start command; update the template string passed to
program.addHelpText to include an example line for start (e.g., "$ jammin start
[options]") so the CLI help shows the start usage alongside the other examples.

bin/cli/src/commands/start-command.ts (1)

36-46: ⚠️ Potential issue | 🔴 Critical

proc.stderr is null when stderr: "inherit" is used.

When stderr: "inherit" is set, Bun pipes stderr directly to the parent process and proc.stderr becomes null. Line 42 attempts to read from proc.stderr, which won't capture error output as intended.

🔧 Option 1: Remove stderr read (simpler, output is already visible)

-  const [stderr, exitCode] = await Promise.all([new Response(proc.stderr).text(), proc.exited]);
+  const exitCode = await proc.exited;

   if (exitCode !== 0) {
-    throw new Error(`Failed to pull image ${DOCKER_IMAGE} with exit code ${exitCode}: ${stderr}`);
+    throw new Error(`Failed to pull image ${DOCKER_IMAGE} with exit code ${exitCode}`);
   }

🔧 Option 2: Capture stderr for error message

   const proc = Bun.spawn(["docker", "pull", "--platform=linux/amd64", DOCKER_IMAGE], {
     stdout: "inherit",
-    stderr: "inherit",
+    stderr: "pipe",
     cwd: process.cwd(),
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 36 - 46, The code reads
proc.stderr even though Bun.spawn was called with stderr: "inherit", which makes
proc.stderr null; update the Bun.spawn call in start-command.ts so stderr is
"pipe" (or remove the stderr read and its use) and then await Promise.all([new
Response(proc.stderr).text(), proc.exited]) to capture the error text;
specifically modify the Bun.spawn options for the proc created in this file (the
variable proc that pulls DOCKER_IMAGE) to use stderr: "pipe" if you want to
include stderr in the thrown Error, or remove the new
Response(proc.stderr).text() read and adjust the error path to rely on exitCode
only if you choose the simpler option.

bin/cli/src/commands/start-command.ts (1)

75-81: Signal handlers are registered but never removed.

The cleanup function is added to SIGINT, SIGTERM, and exit events but is never removed after the container exits. In long-running processes or when startContainer is called multiple times, this could lead to listener accumulation.

♻️ Proposed fix to remove listeners after use

   const cleanup = () => {
     proc.kill();
   };

   process.on("SIGINT", cleanup);
   process.on("SIGTERM", cleanup);
   process.on("exit", cleanup);

-  await proc.exited;
+  try {
+    await proc.exited;
+  } finally {
+    process.off("SIGINT", cleanup);
+    process.off("SIGTERM", cleanup);
+    process.off("exit", cleanup);
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 75 - 81, The signal
handlers registered with process.on("SIGINT"/"SIGTERM"/"exit") use the cleanup
function (which calls proc.kill()) but are never removed, causing listener
accumulation; update the cleanup to be idempotent and, after the
container/process finishes, call process.off("SIGINT", cleanup),
process.off("SIGTERM", cleanup) and process.off("exit", cleanup) (or
removeListener) so listeners are removed, or alternatively remove those
listeners inside cleanup once it runs; reference the existing cleanup function
and the proc variable when making changes.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

bin/cli/src/commands/start-command.ts (1)

36-46: ⚠️ Potential issue | 🔴 Critical

stderr is unavailable when set to "inherit" — reading proc.stderr will fail or return empty.

Bun.spawn with stderr: "inherit" pipes stderr directly to the parent, making proc.stderr null. The new Response(proc.stderr).text() on line 42 won't capture meaningful output.

Either change to stderr: "pipe" to capture the error text, or drop the stderr read since output is already visible to the user:

Option A: Capture stderr for the error message

   const proc = Bun.spawn(["docker", "pull", "--platform=linux/amd64", DOCKER_IMAGE], {
     stdout: "inherit",
-    stderr: "inherit",
+    stderr: "pipe",
     cwd: process.cwd(),
   });

Option B: Drop the stderr read (simpler)

-  const [stderr, exitCode] = await Promise.all([new Response(proc.stderr).text(), proc.exited]);
+  const exitCode = await proc.exited;

   if (exitCode !== 0) {
-    throw new Error(`Failed to pull image ${DOCKER_IMAGE} with exit code ${exitCode}: ${stderr}`);
+    throw new Error(`Failed to pull image ${DOCKER_IMAGE} with exit code ${exitCode}`);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 36 - 46, The pull logic
reads proc.stderr but sets stderr: "inherit", so proc.stderr is null; update the
Bun.spawn call in start-command.ts (where proc and DOCKER_IMAGE are used) to
either change stderr to "pipe" and keep reading new Response(proc.stderr).text()
for inclusion in the thrown Error, or remove the stderr read entirely and rely
on the inherited output and exitCode to throw a simpler Error; modify the
Bun.spawn options and the subsequent Promise.all/read of proc.stderr accordingly
so you don't attempt to read a null proc.stderr.

bin/cli/index.ts (1)

29-38: Help text examples don't include the new start command.

The examples section lists new, build, test, and deploy but omits start. I see there's already a TODO on line 28 about this, so this is just a heads-up.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/index.ts` around lines 29 - 38, The help examples passed to
program.addHelpText are missing the new start command; update the string literal
supplied to program.addHelpText to include a line showing the start command
(e.g., "$ jammin start [options]") alongside the existing examples so the CLI
help reflects the new start command (refer to program.addHelpText where the
Examples block is defined).

bin/cli/src/commands/start-command.ts (3)

17-30: Missing JSDoc comment on exported function.

createFilteredGenesis is exported and public-facing but lacks a JSDoc comment, unlike the other exported functions in this file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 17 - 30, Add a JSDoc
block above the exported function createFilteredGenesis describing its purpose,
parameters, return value, and behavior: note that it reads a genesis JSON from
genesisPath, removes keys listed in GENESIS_STATE_DROP_KEYS from
genesis.genesis_state (if present), writes the filtered JSON to a temporary file
(named with jammin-genesis-<timestamp>.json) and returns the temp file path as a
Promise<string>; include `@param` {string} genesisPath and `@returns`
{Promise<string>} and mention potential side effects (creates temp file) and
thrown errors when parsing or writing fail so the public API is documented.

---

75-83: Signal listeners are never removed and proc.kill() may throw after exit.

After proc.exited resolves, the SIGINT/SIGTERM/exit listeners remain attached. If the CLI process continues (or startContainer is called again), listeners accumulate. Additionally, calling proc.kill() on an already-exited process can throw.

Suggested fix: remove listeners after exit and guard proc.kill

   const cleanup = () => {
-    proc.kill();
+    try { proc.kill(); } catch {}
   };

   process.on("SIGINT", cleanup);
   process.on("SIGTERM", cleanup);
   process.on("exit", cleanup);

   await proc.exited;
+
+  process.off("SIGINT", cleanup);
+  process.off("SIGTERM", cleanup);
+  process.off("exit", cleanup);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` around lines 75 - 83, The cleanup
listener currently registered for "SIGINT"/"SIGTERM"/"exit" should be removed
after awaiting proc.exited and proc.kill should be called safely; update the
code around the cleanup function and the process.on registrations to (1) ensure
cleanup removes the three listeners (use process.off/removeListener for the same
handler) once proc.exited resolves and (2) guard the call to proc.kill so it
won’t throw if the child already exited (either check a boolean like proc.killed
or wrap proc.kill in a try/catch). Reference the existing symbols: cleanup,
proc.kill, and proc.exited when making these changes.

---

53-53: Use join() for path construction instead of string interpolation.

Line 53 uses template-literal concatenation for the logs path, while join from node:path is already imported and used elsewhere in this file. This is inconsistent and won't handle edge cases (e.g., trailing slashes on cwd()).

Suggested fix

-  const logsPath = `${process.cwd()}/logs`;
+  const logsPath = join(process.cwd(), "logs");

Note: You'll need to add join to the import from node:path on line 3 — it's already imported there.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.ts` at line 53, Replace the
string-interpolated logsPath (`const logsPath = `${process.cwd()}/logs`;`) with
a proper path.join call to handle edge cases: use join(process.cwd(), 'logs')
for the logsPath variable (ensure join is imported from 'node:path' alongside
other imports); update the declaration of logsPath to reference join so path
separators and trailing slashes are handled correctly.

bin/cli/src/commands/start-command.test.ts (1)

138-141: Double invocation of pullImage() in error test — consider consolidating.

Each expect(pullImage()).rejects.toThrow(...) call invokes pullImage() separately, triggering two mock spawn calls. While the return Promise.all(...) pattern correctly awaits the assertions (addressing the past review about missing await), you could combine both substring checks in a single call for clarity:

Suggested simplification

-      return Promise.all([
-        expect(pullImage()).rejects.toThrow("Failed to pull image"),
-        expect(pullImage()).rejects.toThrow("exit code 1"),
-      ]);
+      const err = await pullImage().catch((e: Error) => e);
+      expect(err).toBeInstanceOf(Error);
+      expect((err as Error).message).toContain("Failed to pull image");
+      expect((err as Error).message).toContain("exit code 1");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/cli/src/commands/start-command.test.ts` around lines 138 - 141, The test
currently calls pullImage() twice inside Promise.all which triggers two mock
spawn calls; change it to invoke pullImage() once, await the single rejected
promise (e.g., await expect(pullImage()).rejects.toThrow(...) or await
expect(pullImage()).rejects.toThrowErrorMatching(/Failed to pull image.*exit
code 1/)) and assert both substrings/regex against that single error — update
the assertions around pullImage() in start-command.test.ts so only one call to
pullImage() is made and both error conditions are checked on that single
rejected result.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro